Conversation
Implements #684 ## Changes ### Core Implementation **bitcoin_price.rs:** - Added publish_rates_to_nostr() method - Publishes NIP-33 event (kind 30078) after fetching from Yadio API - Transforms rates to format: {"USD": {"BTC": 0.000024}, ...} - Non-blocking: logs error but doesn't fail update if Nostr publish fails **nip33.rs:** - Added new_exchange_rates_event() function - Creates addressable event with d tag "rates" - Includes updated_at and source tags **config/constants.rs:** - Added NOSTR_EXCHANGE_RATES_EVENT_KIND constant (30078) **config/types.rs:** - Added publish_exchange_rates_to_nostr boolean config (default: true) - Enabled by default for censorship resistance ### Event Structure Kind: 30078 (NIP-33 addressable) d tag: "rates" Content: JSON object mapping currencies to BTC rates Tags: updated_at (unix timestamp), source ("yadio") ### Benefits ✅ Censorship-resistant (works in Venezuela, Cuba, censored regions) ✅ Zero scaling cost (relays host events for free) ✅ Backward compatible (HTTP API remains available) ✅ Non-invasive (feature can be disabled via config) ### Security - Events signed with Mostro's private key - Clients MUST verify event pubkey matches connected Mostro instance - Prevents price manipulation attacks from malicious actors ### Testing - Unit tests for rate format transformation - Unit tests for JSON serialization - Integration test: cargo check passes ### Documentation - Added docs/NOSTR_EXCHANGE_RATES.md with: - Event structure and format - Configuration guide - Security considerations - Testing instructions - Future enhancements ### Deployment - Default enabled (publish_exchange_rates_to_nostr = true) - Update frequency: 5 minutes (same as existing price job) - Error handling: best-effort publishing, logs failures ## Related - Issue #684: Feature proposal - Mobile client spec: MostroP2P/app/.specify/NOSTR_EXCHANGE_RATES.md - NIP-33: Parameterized Replaceable Events
Makes the event identifier more specific to Mostro to avoid potential conflicts with other exchange rate providers on Nostr. Updated: - nip33.rs: new_exchange_rates_event() now uses 'mostro-rates' - docs/NOSTR_EXCHANGE_RATES.md: all examples updated
Added exchange_rates_update_interval_seconds config option: **Config (types.rs):** - Added exchange_rates_update_interval_seconds field (default: 300s = 5 min) - Added default_exchange_rates_update_interval() helper **Scheduler (scheduler.rs):** - job_update_bitcoin_prices() now reads interval from config - Logs interval on startup: 'Starting Bitcoin price update job (interval: Xs)' **Bug fixes (bitcoin_price.rs):** - Fixed RwLock lifetime issue: drop lock before await - Fixed Tags::new() → Tags::from_list() - Fixed send_event() signature (pass &event, not event.clone()) - Use debug formatting for send_event output **Documentation (docs/NOSTR_EXCHANGE_RATES.md):** - Added exchange_rates_update_interval_seconds config example - Documented recommended values (production: 300s, dev: 60s, low-volume: 600s) - Added interval config to deployment checklist - Warning about API rate limits for short intervals **Example settings.toml:** ```toml [mostro] publish_exchange_rates_to_nostr = true exchange_rates_update_interval_seconds = 300 # 5 minutes (default) ``` **Testing:** ✅ cargo check passes ✅ Configurable interval reduces API calls for low-traffic instances ✅ Default value maintains backward compatibility
Added expiration tag to prevent stale rates living forever on relays: **Implementation (bitcoin_price.rs):** - Calculate expiration timestamp: current_time + 3600 seconds (1 hour) - Add Tag::expiration() to event tags - Relays supporting NIP-40 will delete events after expiration **Why 1 hour?** - Exchange rates update every 5 minutes (default) - 1 hour provides ~12x redundancy for client sync delays - Prevents old rates from polluting relay storage - Clients should always fetch fresh rates on app launch **Documentation updates:** - Added expiration tag to event structure examples - Documented NIP-40 behavior in Relay Security section **Code quality:** ✅ cargo fmt (formatted) ✅ cargo clippy --all-targets --all-features -D warnings (passes) - Fixed unused variable warning in map_err closure **Testing:** ✅ Compiles without warnings ✅ Expiration tag correctly formatted as unix timestamp
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request adds Nostr NIP-33 publication of Bitcoin/fiat exchange rates to the Mostro daemon. The feature includes new configuration options, rate formatting/serialization logic, event construction helpers, configurable update intervals, and comprehensive documentation detailing event structure, content schema, and verification requirements. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant BitcoinPriceManager
participant Yadio
participant NIP33Event
participant NostrRelays
Scheduler->>BitcoinPriceManager: update_prices()
BitcoinPriceManager->>Yadio: fetch exchange rates
Yadio-->>BitcoinPriceManager: rates (HashMap)
BitcoinPriceManager->>BitcoinPriceManager: cache rates (write lock)
alt publish_exchange_rates_to_nostr enabled
BitcoinPriceManager->>BitcoinPriceManager: transform to nested JSON
BitcoinPriceManager->>NIP33Event: new_exchange_rates_event(content, tags)
NIP33Event->>NIP33Event: create & sign event (kind 30078)
NIP33Event-->>BitcoinPriceManager: signed Event
BitcoinPriceManager->>NostrRelays: send event
NostrRelays-->>BitcoinPriceManager: Ok / Err
BitcoinPriceManager->>BitcoinPriceManager: log result (non-fatal)
end
BitcoinPriceManager-->>Scheduler: Ok / Err
Scheduler->>Scheduler: sleep(interval_seconds)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/NOSTR_EXCHANGE_RATES.md`:
- Around line 100-101: Update the docs to reflect that the scheduler cadence is
configurable: replace the fixed "runs every 300 seconds" statement for
job_update_bitcoin_prices() with a note that the job uses the configurable
exchange_rates_update_interval_seconds value (document where that config is
set/overridden) and mention that BitcoinPriceManager in bitcoin_price.rs
respects this interval; ensure the README entry for Scheduler (scheduler.rs)
references exchange_rates_update_interval_seconds rather than a hardcoded 300s.
- Around line 195-204: The fenced code blocks containing the log snippets (e.g.,
lines showing "INFO Exchange rates published to Nostr. Event ID: <id> (<N>
currencies)" and the "ERROR Failed to publish exchange rates to Nostr: <error>"
/ "ERROR Failed to send exchange rates event to relays: <error>" lines) are bare
fences and must include a language identifier to satisfy markdownlint; update
each triple-backtick fence around these log examples to use a language specifier
such as text (e.g., ```text) so the blocks become fenced with a language.
In `@src/bitcoin_price.rs`:
- Around line 84-93: The expiration is hard-coded to one hour and can outlive
the configured scheduler; update the code that builds Tags (Tags::from_list) to
compute expiration = timestamp +
min(config.exchange_rates_update_interval_seconds, MAX_INTERVAL) (or validate
and return an error if config.exchange_rates_update_interval_seconds >= 3600)
and use Tag::expiration(Timestamp::from(expiration as u64)) instead of the fixed
3600; reference the timestamp variable and Tag::expiration/Timestamp::from so
the expiry is derived from exchange_rates_update_interval_seconds (or
rejected/capped) to ensure events do not expire before the next publish.
- Around line 71-82: The current transformation in formatted_rates (mapping
rates.iter() -> (currency.clone(), rate_map) with rate_map.insert("BTC",
*btc_rate)) preserves upstream numeric values but the published contract expects
fiat→BTC (e.g., "USD": {"BTC": 0.000024}), so invert the quote here: when
building rate_map in the map closure for formatted_rates, insert 1.0 / btc_rate
instead of *btc_rate and guard against zero (return an error or skip/handle zero
to avoid division-by-zero). Update any error paths that may be affected (the
serde_json::to_string &
MostroInternalErr(ServiceError::MessageSerializationError) usage can remain
unchanged).
- Around line 105-117: The update_prices() path currently awaits
client.send_event(&event).await directly which can block the price refresh loop;
wrap that call in a timeout (e.g., tokio::time::timeout) and handle the timeout
error by logging and continuing, or publish from a detached task (tokio::spawn)
so the main refresh loop isn't stalled. Specifically, modify the call site in
update_prices() where client.send_event(&event).await is used: either replace
the await with tokio::time::timeout(Duration::from_secs(<reasonable>),
client.send_event(&event)).await and treat a timeout as a non-fatal error, or
move client.send_event(event) into tokio::spawn(async move { let _ =
client.send_event(&event).await; /* log errors */ }); ensuring you still log
success/failure without blocking the scheduler.
In `@src/nip33.rs`:
- Around line 157-168: The example in src/nip33.rs serializes a flat
HashMap<String, f64> which produces {"USD":0.000024} but the API expects a
nested map like {"USD":{"BTC":0.000024}}; update the example payload used with
new_exchange_rates_event so that the rates variable is a HashMap<String,
HashMap<String, f64>> (or otherwise construct the nested map) and serialize that
nested structure into content before calling new_exchange_rates_event, leaving
the Tag/Tags/TagKind usage unchanged.
In `@src/scheduler.rs`:
- Around line 471-484: Validate and clamp the configured update interval before
entering the loop: read Settings::get_mostro() and the update_interval, then if
update_interval == 0 (or below a minimum like 5 or 60 seconds) log an
error/warning and set a safe default or return an Err to avoid spinning; use the
validated value when calling tokio::time::Duration::from_secs(...) in the loop
that calls BitcoinPriceManager::update_prices(), so a zero-second config cannot
cause continuous tight looping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 869e0ce7-64fd-4bf9-8e28-6079847a3e27
📒 Files selected for processing (6)
docs/NOSTR_EXCHANGE_RATES.mdsrc/bitcoin_price.rssrc/config/constants.rssrc/config/types.rssrc/nip33.rssrc/scheduler.rs
src/bitcoin_price.rs
Outdated
| // Transform rates to expected format: {"USD": {"BTC": 0.000024}, ...} | ||
| let formatted_rates: HashMap<String, HashMap<String, f64>> = rates | ||
| .iter() | ||
| .map(|(currency, btc_rate)| { | ||
| let mut rate_map = HashMap::new(); | ||
| rate_map.insert("BTC".to_string(), *btc_rate); | ||
| (currency.clone(), rate_map) | ||
| }) | ||
| .collect(); | ||
|
|
||
| let content = serde_json::to_string(&formatted_rates) | ||
| .map_err(|_| MostroInternalErr(ServiceError::MessageSerializationError))?; |
There was a problem hiding this comment.
Make the quote direction explicit.
This mapping preserves the upstream numeric value verbatim. In the same file, the Yadio fixture is BTC.USD = 50000.0, while the published-event contract/docs describe USD -> {"BTC": 0.000024}. Unless the upstream response is already fiat→BTC, clients will receive the inverse quote. Either invert here or align the wire contract/tests/docs around BTC→fiat.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bitcoin_price.rs` around lines 71 - 82, The current transformation in
formatted_rates (mapping rates.iter() -> (currency.clone(), rate_map) with
rate_map.insert("BTC", *btc_rate)) preserves upstream numeric values but the
published contract expects fiat→BTC (e.g., "USD": {"BTC": 0.000024}), so invert
the quote here: when building rate_map in the map closure for formatted_rates,
insert 1.0 / btc_rate instead of *btc_rate and guard against zero (return an
error or skip/handle zero to avoid division-by-zero). Update any error paths
that may be affected (the serde_json::to_string &
MostroInternalErr(ServiceError::MessageSerializationError) usage can remain
unchanged).
CRITICAL BUG FIX: Rates were being published in inverted format.
**Problem:**
- Yadio API returns: {"USD": 50000.0} (price of 1 BTC in USD)
- We were transforming to: {"USD": {"BTC": 0.000024}} (USD per BTC)
- This inverted the rate semantics completely
- Clients would receive wrong prices (e.g. 0.000024 instead of 50000)
**Root Cause:**
Misunderstood the Yadio API response format. The values are already
the price of 1 BTC in that fiat currency, NOT the amount of BTC per
fiat unit.
**Fix:**
- Remove unnecessary transformation
- Publish rates as-is from Yadio: {"USD": 50000.0, "EUR": 45000.0, ...}
- This matches what clients expect: price of 1 BTC in fiat
**Changes:**
- bitcoin_price.rs: Removed HashMap<String, HashMap<String, f64>> transformation
- bitcoin_price.rs: Updated tests to reflect correct format
- docs/NOSTR_EXCHANGE_RATES.md: Fixed examples and semantics
- .specify/NOSTR_EXCHANGE_RATES.md: Fixed client spec examples
**Testing:**
✅ cargo test bitcoin_price::tests (all pass)
✅ Rate semantics now correct: "USD": 50000.0 = 1 BTC costs 50,000 USD
**Impact:**
- This was a showstopper bug — clients would get completely wrong prices
- Must be fixed before merge
Implemented all inline review suggestions: **1. Docs: Scheduler cadence configurable** - Updated NOSTR_EXCHANGE_RATES.md to reference configurable interval - Changed "runs every 300 seconds" to "runs at intervals configured by exchange_rates_update_interval_seconds" **2. Docs: Add language identifiers to code fences** - Added ```text to log snippet fences for markdownlint compliance **3. bitcoin_price.rs: Dynamic expiration based on config** - Expiration now calculated as min(update_interval * 2, 3600) - Ensures events don't expire before next publish - Caps at 1 hour to prevent stale data **4. bitcoin_price.rs: Timeout for send_event** - Wrapped client.send_event in tokio::time::timeout (30s) - Prevents blocking the scheduler if relay is unresponsive - Logs timeout errors separately **5. scheduler.rs: Validate minimum interval** - Added MIN_INTERVAL constant (60s) to prevent API rate limit abuse - Logs error if configured value too low and uses minimum - Prevents tight-loop spinning on misconfiguration **6. nip33.rs: Updated example docstring** - Fixed example to show HashMap<String, f64> (flat structure) - Added comment explaining Yadio format - Included expiration tag in example **Code quality:** ✅ cargo fmt (formatted) ✅ cargo clippy --all-targets --all-features -D warnings (passes) ✅ cargo check (compiles) **Note on rate format:** Keeping current format (flat HashMap) as it correctly represents "price of 1 BTC in fiat". CodeRabbit's suggestion to invert (1.0 / btc_rate) would break the semantics — we want USD=50000 (1 BTC costs 50k USD), not USD=0.00002 (1 USD buys 0.00002 BTC).
BLOCKER 1: Unify content format across code and docs
- Fixed last occurrence in docs (jq example)
- Fixed docstring comment in nip33.rs
- All docs now show flat format: {"USD": 50000.0, "EUR": 45000.0}
- Consistent with code implementation
BLOCKER 2: Rename updated_at → published_at
- Tag name now accurately reflects semantics
- published_at = daemon publish time (not source timestamp)
- Updated everywhere: code, docs, examples, client spec
- Clarified in docs: "Unix timestamp when daemon published the event"
MINOR 3: Make publish_rates_to_nostr truly best-effort
- All error paths now return Ok(()) after logging
- Timeout errors no longer propagate up
- Comment added: "Best-effort: log errors but don't fail update job"
- Matches the "non-blocking" behavior described in docs
Changes:
- src/bitcoin_price.rs: published_at tag, best-effort error handling
- src/nip33.rs: published_at in docstring example
- docs/NOSTR_EXCHANGE_RATES.md: published_at tag, flat format examples
- .specify/NOSTR_EXCHANGE_RATES.md: published_at tag
Code quality:
✅ cargo fmt
✅ cargo clippy -D warnings (passes)
Ready for merge.
Changed content structure to match Yadio API response exactly:
**Before (flat):**
{"USD": 50000.0, "EUR": 45000.0, ...}
**After (Yadio format):**
{"BTC": {"USD": 50000.0, "EUR": 45000.0, ...}}
**Why:**
- Clients can parse it identically to Yadio HTTP API responses
- No transformation needed on client side
- Includes all currencies Yadio publishes (not filtered)
- Format is self-documenting (BTC wrapper makes it clear)
**Changes:**
- src/bitcoin_price.rs: Wrap rates in {"BTC": ...} structure
- src/bitcoin_price.rs: Updated tests to verify wrapper
- src/nip33.rs: Updated docstring example
- docs/NOSTR_EXCHANGE_RATES.md: All examples show Yadio format
- .specify/NOSTR_EXCHANGE_RATES.md: Client spec updated
**Testing:**
✅ cargo test bitcoin_price::tests (all pass)
✅ cargo fmt
✅ cargo clippy -D warnings (passes)
**Example event content:**
{
"BTC": {
"BTC": 1,
"USD": 50000.0,
"EUR": 45000.0,
"ARS": 105000000.0,
"VES": 850000000.0,
...all Yadio currencies...
}
}
**BLOCKER 1: BTC key collision in test**
- Removed "BTC": 1.0 from input_rates in test_rates_json_serialization
- Now uses only fiat currencies (USD, EUR) like real Yadio response
- Added assertion to ensure no nested "BTC":1 in output
- Prevents test from normalizing invalid format
Why: Yadio includes "BTC" as the wrapper key, not inside the rates map.
The old test was creating {"BTC": {"BTC": 1.0, "USD": 50000.0}}, which
would never occur in production and could hide bugs.
**MINOR 2: Clarify Settings::get_mostro() double read**
- Added comment explaining why we read settings in publish_rates_to_nostr()
- Rationale: Ensures expiration stays aligned with interval if config reloaded
- Not a performance concern (settings read is cheap, publish is best-effort)
**Testing:**
✅ cargo test bitcoin_price::tests::test_rates_json_serialization (passes)
✅ cargo fmt
✅ cargo clippy -D warnings (passes)
Ready for merge.
Add documentation for the new exchange rates publishing feature via Nostr.
## Overview
Mostro instances can now publish Bitcoin/fiat exchange rates as NIP-33
addressable events (kind 30078) to provide censorship-resistant price
data for clients.
## Key Features
- **Event kind**: 30078 (NIP-33 addressable, replaceable)
- **d tag**: "mostro-rates" (unique identifier)
- **Content**: Full Yadio API format {\BTC\: {\USD\: 50000.0, ...}}
- **Expiration**: NIP-40 tag (min(interval × 2, 3600))
- **Configurable**: Update interval (default 300s, min 60s)
- **Optional**: Can be disabled via config
## Why Nostr?
In censored regions (Venezuela, Cuba, Iran), HTTP exchange rate APIs
are often blocked. Nostr's decentralized relay network provides:
- Censorship resistance
- Zero scaling cost
- Geographic distribution
- No single point of failure
## Security
Clients MUST:
- Verify event.pubkey matches expected Mostro instance
- Verify signature (NIP-01)
- Check expiration timestamp
- Fall back to HTTP API if Nostr fails
## Implementation
- Daemon PR: MostroP2P/mostro#685
- Client spec: MostroP2P/app#36
Related to the Venezuela censorship resistance use case discussed in:
MostroP2P/mostro#684
Implements #684
Summary
Mostro daemon now publishes Bitcoin/fiat exchange rates to Nostr relays as NIP-33 addressable events (kind
30078), enabling censorship-resistant rate fetching for mobile clients.Changes
Core Implementation
bitcoin_price.rs
publish_rates_to_nostr()method{"USD": {"BTC": 0.000024}, ...}nip33.rs
new_exchange_rates_event()function"rates"updated_at(unix timestamp) andsource("yadio") tagsconfig/constants.rs
NOSTR_EXCHANGE_RATES_EVENT_KINDconstant (30078)config/types.rs
publish_exchange_rates_to_nostrboolean configtrue(enabled for censorship resistance)Event Structure
{ "kind": 30078, "pubkey": "<mostro_pubkey>", "created_at": 1732546800, "tags": [ ["d", "rates"], ["updated_at", "1732546800"], ["source", "yadio"] ], "content": "{\"USD\": {\"BTC\": 0.000024}, \"EUR\": {\"BTC\": 0.000022}, ...}", "sig": "..." }d tag:
"rates"(NIP-33 identifier — replaces previous rate events)Content format:
{"CURRENCY": {"BTC": rate}, ...}Benefits
Security
Event Signing
Client-Side Verification (Critical)
Mobile clients MUST verify
event.pubkeymatches their connected Mostro instance to prevent price manipulation attacks.Attack scenario: Malicious actor publishes fake rates to influence order creation.
Mitigation: Clients only accept events signed by their connected Mostro instance.
See: Mobile client spec
Configuration
Enable/Disable
Add to
settings.toml:Update Frequency
5 minutes (same as existing
job_update_bitcoin_prices()interval)Testing
Unit Tests
cargo test bitcoin_priceCoverage:
{"USD": 0.024}→{"USD": {"BTC": 0.024}})Integration Testing
publish_exchange_rates_to_nostr = true30078events:Expected output: JSON event with current exchange rates
Documentation
Added
docs/NOSTR_EXCHANGE_RATES.mdwith:Deployment
Production Checklist
publish_exchange_rates_to_nostrconfig insettings.tomlMonitoring
Success:
Error:
Related
Future Enhancements
Breaking Changes
None. Feature is additive and can be disabled via config.
Summary by CodeRabbit